COPY TO (FREEZE)?

  • Jump to comment-1
    horikyota.ntt@gmail.com2022-08-02T04:30:46+00:00
    I noticed that COPY TO accepts FREEZE option but it is pointless. Don't we reject that option as the first-attached does? I tempted to add tests for those option combinations that are to be rejected but I didin't come up with a clean way to do that. By the way, most of the invalid option combinations for COPY are marked as ERRCODE_FEATURE_NOT_SUPPORTED. I looks to me saying that "that feature is theoretically possible or actually realized elsewhere, but impossible now or here". If it is correct, aren't they better be ERRCODE_INVALID_PARAMETER_VALUE? The code is being used for similar messages "unrecognized parameter <name>" and "parameter <name> specified more than once" (or some others?). At least a quote string longer than a single character seems like to fit INVALID_PARAMETER_VALUE. (I believe we don't mean to support multicharacter (or even multibyte) escape/quote character anddelimiter). That being said, I'm not sure if the change will be worth the trouble. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
    • Jump to comment-1
      zmlpostgres@gmail.com2022-08-02T08:20:29+00:00
      Regards, Zhang Mingli On Aug 2, 2022, 12:30 +0800, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, wrote: > I noticed that COPY TO accepts FREEZE option but it is pointless. > > Don't we reject that option as the first-attached does? I +1, should be rejected like other invalid options. > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center
    • Jump to comment-1
      rjuju123@gmail.com2022-08-02T06:17:46+00:00
      Hi, On Tue, Aug 02, 2022 at 01:30:46PM +0900, Kyotaro Horiguchi wrote: > I noticed that COPY TO accepts FREEZE option but it is pointless. > > Don't we reject that option as the first-attached does? I agree that we should reject it, +1 for the patch. > By the way, most of the invalid option combinations for COPY are > marked as ERRCODE_FEATURE_NOT_SUPPORTED. I looks to me saying that > "that feature is theoretically possible or actually realized > elsewhere, but impossible now or here". > > If it is correct, aren't they better be ERRCODE_INVALID_PARAMETER_VALUE? The > code is being used for similar messages "unrecognized parameter <name>" and > "parameter <name> specified more than once" (or some others?). At least a > quote string longer than a single character seems like to fit > INVALID_PARAMETER_VALUE. (I believe we don't mean to support multicharacter > (or even multibyte) escape/quote character anddelimiter). That being said, > I'm not sure if the change will be worth the trouble. I also feel weird about it. I raised the same point recently about COPY FROM + HEADER MATCH (1), and at that time there wasn't a real consensus on the way to go, just keep the things consistent. I'm +0.5 on that patch for the same reason as back then. My only concern is that it can in theory break things if you rely on the current sqlstate, but given the errors I don't think it's really a problem. [1]: https://www.postgresql.org/message-id/flat/20220614091319.jk4he5migtpwyd7r%40jrouhaud#b18bf3705fb9f69d0112b6febf0fa1be
      • Jump to comment-1
        horikyota.ntt@gmail.com2022-08-02T08:17:35+00:00
        At Tue, 2 Aug 2022 14:17:46 +0800, Julien Rouhaud <rjuju123@gmail.com> wrote in > Hi, > > On Tue, Aug 02, 2022 at 01:30:46PM +0900, Kyotaro Horiguchi wrote: > > I noticed that COPY TO accepts FREEZE option but it is pointless. > > > > Don't we reject that option as the first-attached does? > > I agree that we should reject it, +1 for the patch. Thanks for looking it! > > By the way, most of the invalid option combinations for COPY are > > marked as ERRCODE_FEATURE_NOT_SUPPORTED. I looks to me saying that > > "that feature is theoretically possible or actually realized > > elsewhere, but impossible now or here". > > > > If it is correct, aren't they better be ERRCODE_INVALID_PARAMETER_VALUE? The > > code is being used for similar messages "unrecognized parameter <name>" and > > "parameter <name> specified more than once" (or some others?). At least a > > quote string longer than a single character seems like to fit > > INVALID_PARAMETER_VALUE. (I believe we don't mean to support multicharacter > > (or even multibyte) escape/quote character anddelimiter). That being said, > > I'm not sure if the change will be worth the trouble. > > I also feel weird about it. I raised the same point recently about COPY FROM + > HEADER MATCH (1), and at that time there wasn't a real consensus on the way to > go, just keep the things consistent. I'm +0.5 on that patch for the same > reason as back then. My only concern is that it can in theory break things if > you rely on the current sqlstate, but given the errors I don't think it's > really a problem. Exactly. That is the exact reason for my to say "I'm not sure if..". > [1]: https://www.postgresql.org/message-id/flat/20220614091319.jk4he5migtpwyd7r%40jrouhaud#b18bf3705fb9f69d0112b6febf0fa1be > Maybe that's just me but I understand "not supported" as "this makes > sense, but this is currently a limitation that might be lifted > later". FWIW I understand it the same way. regards. -- Kyotaro Horiguchi NTT Open Source Software Center